Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch Pulpcore testing to using puppet-pulpcore test suite #1729

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Nov 15, 2023

Given we install Pulpcore using puppet-pulpcore, the idea here is to vet packaging changes against the puppet module rather than the Pulp Ansible installer.

This is in draft mode because first the puppet module needs to support configuring the baseurl in order to point the testing at stagingyum.theforeman.org.

BEAKER_HYPERVISOR: "docker"
BEAKER_provision: "yes"
BEAKER_setfile: "centos8-64{hostname=centos8-64.example.com}"
BEAKER_destroy: "no"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not destroying them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes debugging easier if it sticks around. We end up destroying the box anyway so no harm in keeping it.

@@ -1,9 +1,11 @@
forklift_name: "pipe-pulp-{{ pipeline_version }}-{{ pipeline_os }}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you have to define that? it should come from install_base.yml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The should did not pan out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Weird. (I mean, you stopped including it, but still)

args:
chdir: /src/puppet-pulpcore
environment:
BEAKER_HYPERVISOR: "docker"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't particularly like that we spin up a centos box just to run a centos container inside, especially as our primary way of deployment is not a container.

Shouldn't beaker default to using the machine it's running on when we do not set BEAKER_HYPERVIRSOR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue it's not just to run a container inside -- we also install dependencies needed for the puppet module to perform testing: https://github.com/theforeman/forklift/pull/1729/files#diff-dec5007e2a651ace599f5154ceb5187aa75fb8373133bac84a45fd15840fbe9dR19-R41

Especially for our CI, that means we have a clean environment every time (yes, I know CentOS CI starts with a clean box and thus we could shorten this). Locally, this also means we have a clean environment every time. And, it's consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair.

I was also thinking about BEAKER_HYPERVISOR: vagrant which would fit nicely if we wouldn't run this all from inside a vagrant box already.

It's hypervisors all the way down.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could possibly do container-inception as well, however, I was not trying to re-invent the wheel here. Rather my goal was to switch away from the Ansible based Pulp installer and to using our own test suite that we vet a working Pulp against.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today we spin up very expensive bare metal boxes so we can use virtualization. If we don't, it's much cheaper for CentOS CI if we request a VM. So I'd say we either go with BEAKER_HYPERVISOR=vagrant_libvirt so we can also test SELinux (which we can't inside containers) or enhance our Duffy integration so it can request VMs via a parameter.

Today that's hardcoded here:
https://github.com/theforeman/jenkins-jobs/blob/d78f0f3b828444b109105fa5d27927be3f158c31/centos.org/ansible/provision_duffy.yml#L6

Perhaps also a good excuse to address theforeman/foreman-infra#1828.

@evgeni
Copy link
Member

evgeni commented Nov 17, 2023

Overall I think this will work, and is down the line the more meaningful test (as it uses the same codebase as we use for prod deployments later on).

What is the plan wrt older Pulpcore releases? Those do not reside on stagingyum and often can't be tested with puppet-pulpcore master. Should we keep the old pipe as "pulpcore_old" or something, and utilize that for releases before 3.39?

@ehelms
Copy link
Member Author

ehelms commented Nov 17, 2023

What is the plan wrt older Pulpcore releases? Those do not reside on stagingyum and often can't be tested with puppet-pulpcore master. Should we keep the old pipe as "pulpcore_old" or something, and utilize that for releases before 3.39?

That is part of the reason this is in draft :) So I appreciate you having thought it through and thrown out an idea. I think keeping the status quo for prior releases is the easiest option.

@ehelms
Copy link
Member Author

ehelms commented Nov 17, 2023

BTW, I was intending to vet this model on new ground first by doing it with Candlepin if you want to review that first -- #1712

@evgeni
Copy link
Member

evgeni commented Nov 17, 2023

What is the plan wrt older Pulpcore releases? Those do not reside on stagingyum and often can't be tested with puppet-pulpcore master. Should we keep the old pipe as "pulpcore_old" or something, and utilize that for releases before 3.39?

That is part of the reason this is in draft :) So I appreciate you having thought it through and thrown out an idea. I think keeping the status quo for prior releases is the easiest option.

We could also be adventurous.

According to https://github.com/theforeman/forklift/blob/master/vagrant/config/versions.yaml we have only to care for 3.22+, so really 3.22, 3.28 and 3.39 (after this whole change).
Today puppet-pulpcore does support those old versions and given the whole repo-url stuff is merged, we could go and use the current state (before merging the 3.39 changes) as the version to test old releases, and master for 3.39+

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may end up with a circular dependency here. puppet-pulpcore tests its CI against nightly, but you need puppet-pulpcore to pass to promote nightly.

- pipeline_version == 'nightly' or pipeline_version is version('3.28', '>=')
- pipeline_os is defined
- pipeline_os is search("centos8-stream")
- name: Install podman-docker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used beaker-docker a few times with plain podman. How I do it on my Fedora 38 box:

$ systemctl start --user podman.socket
$ export DOCKER_HOST=unix:///run/user/$UID/podman/podman.sock

Does it make sense to introduce a podman role for this?

- libyaml-devel
state: installed

- name: Clone puppet-pulpcore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to add a parameter to select the branch. In git master we may drop support for a version that still has pipelines. If we can then pin it to some stable branch it should still be good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could include the role from #1580 and then set the source repository and any number of desired remotes and branches in pipelines/vars/forklift_pulpcore.yml

That handles the bundle install as well if you add install_gems: true

@evgeni
Copy link
Member

evgeni commented Nov 17, 2023

We may end up with a circular dependency here. puppet-pulpcore tests its CI against nightly, but you need puppet-pulpcore to pass to promote nightly.

We certainly will (even tho in many cases in the past adding a new version was more something optional, as it enabled new features or something), and I guess we won't get away w/o running releases with an unmerged puppet-pulpcore branch for a moment in the case where it's truly needed (like now with 3.39), but this seems "ok"?

(I mean, right now, we point nightly/3.39 at a fork of the old installer pcreech hacked together, so same same)

@ehelms
Copy link
Member Author

ehelms commented Nov 17, 2023

I was thinking that even though it may be circular, it would help identify if new Pulp versions / dependencies arriving via the nightly pipeline introduce a breakage we need to be aware of in puppet-pulpcore as early as possible.

@ekohl
Copy link
Member

ekohl commented Nov 17, 2023

We can work around it, by pointing a PR (temporarily) at staging (theforeman/puppet-pulpcore@fa2bd4f makes that possible). My main issue is that we should acknowledge this by documenting is somewhere.

@Odilhao Odilhao force-pushed the pipeline-puppet-pulpcore branch 3 times, most recently from 4292cc5 to 9c3e413 Compare March 15, 2024 03:22
@Odilhao
Copy link
Member

Odilhao commented Mar 15, 2024

Now that theforeman/puppet-pulpcore#333 was merged, we can look at this PR again.

pipelines/pulpcore/02-install.yml Outdated Show resolved Hide resolved
playbooks/setup_forklift.yml Outdated Show resolved Hide resolved
playbooks/setup_forklift.yml Outdated Show resolved Hide resolved
@Odilhao Odilhao force-pushed the pipeline-puppet-pulpcore branch 2 times, most recently from 8eedbcf to 7a876ba Compare March 15, 2024 15:18
@ehelms ehelms marked this pull request as ready for review March 15, 2024 18:58
@ehelms ehelms merged commit 9fc10f8 into theforeman:master Mar 18, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants